[dd-agent] add multiple enpoints/api_keys conf #317

degemer · 2016-06-27T21:21:59Z

No description provided.

olivielpeau · 2016-07-04T16:40:56Z

spec/dd-agent_spec.rb

+        ) do |node|
+          node.set['datadog'] = {
+            'api_key' => 'somethingnotnil',
+            'other_dd_urls' => ['app.example.com', 'app.datadoghq.com'],


nitpick since it's not really what's tested here: it'd be better to use the full urls with the protocol, i.e. https://app.example.com

olivielpeau · 2016-07-04T16:43:49Z

Added a few nitpicks but this is👌

Let's merge it for 2.5.0

Pass multiple keys to the handler.

olivielpeau · 2016-09-08T12:57:29Z

recipes/dd-agent.rb

-    :api_key => node['datadog']['api_key'],
-    :dd_url => node['datadog']['url']
+    :api_key => api_keys,
+    :dd_url => dd_urls


Wouldn't it make sense to update the var names too?

Indeed. 🎆

olivielpeau · 2016-09-08T13:08:01Z

Added some comments, let me know when the tests are updated ;)

Overall it looks way better now compared to the first draft 🚀

olivielpeau · 2016-09-08T13:45:04Z

attributes/default.rb

@@ -21,9 +21,11 @@
 # The Datadog api key to associate your agent's data with your organization.
 # Can be found here:
 # https://app.datadoghq.com/account/settings
+# This can be a list of api keys


(just to be more explicit, I'd explain here what a list of api keys does, i.e. that it configures the Agent and the handler to send data to multiple accounts)

You're right, it's not clear enough. I also changed list for array, as a list could be understood as a string "apikey1, apikey2".

degemer · 2016-09-08T21:43:50Z

💚 CI finally, ready for final review !

olivielpeau · 2016-09-09T10:02:19Z

attributes/default.rb

@@ -21,9 +21,13 @@
 # The Datadog api key to associate your agent's data with your organization.
 # Can be found here:
 # https://app.datadoghq.com/account/settings
+# This can be an array of api keys (to configure the agent
+# data to send to multiple accounts)


should be to send data to multiple accounts?

olivielpeau · 2016-09-09T10:09:56Z

Added one last comment about a comment, once that's fixed let's !

degemer · 2016-09-09T21:04:15Z

As we discussed, I updated this PR to use a extra_endpoints @olivielpeau. Sorry for the multiple changes, one more review for you!

olivielpeau · 2016-09-13T12:47:48Z

recipes/dd-handler.rb

+extra_endpoints = []
+node['datadog']['extra_endpoints'].each do |_, endpoint|
+  next unless endpoint['enabled']
+  endpoint.delete('enabled')


This will attempt to mutate node['datadog']['extra_endpoints'] too, which chef doesn't allow from a recipe (it'll cause a compile error).

I think we should either copy the endpoint hash first (keeping in mind that it's not a regular hash but an ImmutableMash if I remember correctly) or "manually" assign the interesting fields of endpoint to a new hash.

Also, unless the resulting values of extra_endpoints var are Mashes I think their keys need to be string literals (:url instead of 'url') since that's how we access the values in the handler.

you can disregard my comment on the Mash thing, it's not a problem since in the handler we copy the config to a Mash since v0.5.0 (see DataDog/chef-handler-datadog#50)

olivielpeau · 2016-09-13T12:54:55Z

Thanks, the logic looks even nicer now :)

Had 2 comments on the PR. Since the dd-handler part is not covered by the automated tests let's make sure to test it thoroughly with kitchen.

For backward compatibility purpose, we keep url, api_key & application_key, and add a extra_endpoints attributes to handle the other endpoints.

Or at least a beginning of tests, with multiple endpoints

degemer · 2016-09-13T21:40:59Z

Comments addressed, and a few tests added.
It's a start, because I didn't want to spend too much time on it. We should add more "one day".

olivielpeau

Looks good! Thanks for the adding these tests! 💚

degemer added this to the 2.5.0 milestone Jun 27, 2016

degemer assigned olivielpeau Jun 27, 2016

degemer force-pushed the quentin/multiple-endpoints branch from 3be5a56 to de05ec0 Compare June 27, 2016 21:22

olivielpeau reviewed Jul 4, 2016
View reviewed changes

olivielpeau added the feature label Jul 8, 2016

olivielpeau modified the milestones: 2.6.0, 2.5.0 Aug 3, 2016

[dd-agent] add multiple enpoints/api_keys conf

d96d7ea

degemer force-pushed the quentin/multiple-endpoints branch from de05ec0 to d96d7ea Compare August 16, 2016 20:33

[handler] add multiple endpoints support

4819c38

Pass multiple keys to the handler.

degemer force-pushed the quentin/multiple-endpoints branch 3 times, most recently from d5f7759 to 777a72e Compare August 23, 2016 15:45

olivielpeau reviewed Sep 8, 2016
View reviewed changes

degemer force-pushed the quentin/multiple-endpoints branch from 777a72e to c633cb7 Compare September 8, 2016 21:32

olivielpeau reviewed Sep 9, 2016
View reviewed changes

degemer force-pushed the quentin/multiple-endpoints branch 3 times, most recently from f489e06 to b2e395e Compare September 9, 2016 20:28

degemer force-pushed the quentin/multiple-endpoints branch from b2e395e to d54c9a2 Compare September 9, 2016 21:04

olivielpeau reviewed Sep 13, 2016
View reviewed changes

olivielpeau mentioned this pull request Sep 13, 2016

Add multiple endpoints support DataDog/chef-handler-datadog#87

Merged

Multiple endpoints: easier configuration

0bea55d

For backward compatibility purpose, we keep url, api_key & application_key, and add a extra_endpoints attributes to handle the other endpoints.

degemer force-pushed the quentin/multiple-endpoints branch from d54c9a2 to 02e5766 Compare September 13, 2016 21:12

[tests] add dd-handler spec test

bbe7268

Or at least a beginning of tests, with multiple endpoints

degemer force-pushed the quentin/multiple-endpoints branch from 02e5766 to bbe7268 Compare September 13, 2016 21:20

olivielpeau approved these changes Sep 15, 2016

View reviewed changes

olivielpeau added the ready label Sep 15, 2016

olivielpeau merged commit f78252d into master Sep 19, 2016

olivielpeau deleted the quentin/multiple-endpoints branch September 19, 2016 15:43

mlcooper mentioned this pull request Jun 23, 2017

Possible spurious error from dd-agent when using multiple API keys DataDog/dd-agent#3396

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dd-agent] add multiple enpoints/api_keys conf #317

[dd-agent] add multiple enpoints/api_keys conf #317

degemer commented Jun 27, 2016

olivielpeau Jul 4, 2016

olivielpeau commented Jul 4, 2016

olivielpeau Sep 8, 2016

degemer Sep 8, 2016

olivielpeau commented Sep 8, 2016

olivielpeau Sep 8, 2016

degemer Sep 8, 2016

degemer commented Sep 8, 2016

olivielpeau Sep 9, 2016

olivielpeau commented Sep 9, 2016

degemer commented Sep 9, 2016

olivielpeau Sep 13, 2016

olivielpeau Sep 13, 2016 •

edited

Loading

olivielpeau commented Sep 13, 2016 •

edited

Loading

degemer commented Sep 13, 2016

olivielpeau left a comment

[dd-agent] add multiple enpoints/api_keys conf #317

[dd-agent] add multiple enpoints/api_keys conf #317

Conversation

degemer commented Jun 27, 2016

olivielpeau Jul 4, 2016

Choose a reason for hiding this comment

olivielpeau commented Jul 4, 2016

olivielpeau Sep 8, 2016

Choose a reason for hiding this comment

degemer Sep 8, 2016

Choose a reason for hiding this comment

olivielpeau commented Sep 8, 2016

olivielpeau Sep 8, 2016

Choose a reason for hiding this comment

degemer Sep 8, 2016

Choose a reason for hiding this comment

degemer commented Sep 8, 2016

olivielpeau Sep 9, 2016

Choose a reason for hiding this comment

olivielpeau commented Sep 9, 2016

degemer commented Sep 9, 2016

olivielpeau Sep 13, 2016

Choose a reason for hiding this comment

olivielpeau Sep 13, 2016 • edited Loading

Choose a reason for hiding this comment

olivielpeau commented Sep 13, 2016 • edited Loading

degemer commented Sep 13, 2016

olivielpeau left a comment

Choose a reason for hiding this comment

olivielpeau Sep 13, 2016 •

edited

Loading

olivielpeau commented Sep 13, 2016 •

edited

Loading